Improve doctor.mjs Playwright browser check with distinct failure modes#9
Improve doctor.mjs Playwright browser check with distinct failure modes#9RajjjAryan with Copilot wants to merge 2 commits into
Conversation
Agent-Logs-Url: https://github.com/RajjjAryan/career-copilot/sessions/308f2dcf-d6d4-44d4-b612-d2e11a64f008 Co-authored-by: RajjjAryan <107031370+RajjjAryan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refines doctor.mjs’s Playwright prerequisite check so it distinguishes between a missing Playwright package and a missing Chromium browser binary, and improves debuggability by printing the resolved Chromium executable path on success.
Changes:
- Split
checkPlaywright()into two stages: Playwright package import vs Chromium binary presence. - Add targeted fix instructions for each failure mode (
npm installvsnpx playwright install chromium). - Include
chromium.executablePath()in the success label.
| } catch { | ||
| return { | ||
| pass: false, | ||
| label: 'Playwright package not available', | ||
| fix: 'Run: npm install', |
There was a problem hiding this comment.
The import stage uses a bare catch and always reports "Playwright package not available" with the generic npm install fix. This will misdiagnose other import-time failures (e.g., corrupted install, unsupported platform/architecture, or other runtime errors). Capture the thrown error and only treat it as "package not available" when it’s a module-not-found case (e.g., err.code === 'ERR_MODULE_NOT_FOUND' / message indicates missing playwright); otherwise surface a distinct failure label (ideally including err.message) so users aren’t sent to the wrong fix.
| } catch { | |
| return { | |
| pass: false, | |
| label: 'Playwright package not available', | |
| fix: 'Run: npm install', | |
| } catch (err) { | |
| const errCode = err && typeof err === 'object' ? err.code : undefined; | |
| const errMessage = err instanceof Error ? err.message : String(err); | |
| const isModuleNotFound = | |
| errCode === 'ERR_MODULE_NOT_FOUND' || | |
| /cannot find package ['"]playwright['"]/i.test(errMessage) || | |
| /cannot find module ['"]playwright['"]/i.test(errMessage) || | |
| /module ['"]playwright['"] was not found/i.test(errMessage); | |
| if (isModuleNotFound) { | |
| return { | |
| pass: false, | |
| label: 'Playwright package not available', | |
| fix: 'Run: npm install', | |
| }; | |
| } | |
| return { | |
| pass: false, | |
| label: `Playwright import failed: ${errMessage}`, |
| try { | ||
| const { chromium } = await import('playwright'); | ||
| const execPath = chromium.executablePath(); | ||
| if (existsSync(execPath)) { | ||
| return { pass: true, label: 'Playwright chromium installed' }; | ||
| return { pass: true, label: `Playwright Chromium browser installed (${execPath})` }; | ||
| } | ||
| return { | ||
| pass: false, | ||
| label: 'Playwright chromium not installed', | ||
| label: 'Playwright Chromium browser not installed', | ||
| fix: 'Run: npx playwright install chromium', | ||
| }; | ||
| } catch { | ||
| return { | ||
| pass: false, | ||
| label: 'Playwright chromium not installed', | ||
| label: 'Playwright Chromium browser not installed', | ||
| fix: 'Run: npx playwright install chromium', | ||
| }; |
There was a problem hiding this comment.
The Chromium check’s catch treats any exception as "browser not installed". If chromium.executablePath() throws for reasons other than a missing browser (e.g., Playwright internal error), the output will point to npx playwright install chromium even though that may not resolve it. Capture the error and report a separate failure mode (or include err.message) so this stage only claims "not installed" when the binary is actually absent.
|
Closing — superseded by #31 which was merged to main and includes a more comprehensive fix (Playwright browser launch test + YAML validation for profile.yml and portals.yml). Thank you for the contribution! 🙏 |
checkPlaywright()treated "playwright package missing" and "chromium binary missing" as the same failure with the same fix instruction, which is misleading.npm installvsnpx playwright install chromium)executablePath()in the pass label for debuggability